-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat/592: Validate length and checksum of fetched masp param #1143
Conversation
97b7792
to
e160a31
Compare
e160a31
to
1167963
Compare
1167963
to
a0830bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
apps/namadillo/src/hooks/useSdk.tsx
Outdated
@@ -13,7 +13,21 @@ import { | |||
} from "react"; | |||
import Proxies from "../../scripts/proxies.json"; | |||
|
|||
export const SdkContext = createContext<Sdk | undefined>(undefined); | |||
export enum MaspParamsStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, but more an idea. What do you think of using QueryStatus type from @tanstack/react-query
instead? So we can make this loading state type standard across the app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
bytes: Uint8Array; | ||
}; | ||
|
||
const MASP_PARAM_ATTR: Record< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment telling from where these numbers / hashes come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, thanks!
33fcb8e
to
43b7a66
Compare
43b7a66
to
1808464
Compare
Resolves #592
This PR validates the fetched MASP param before storing it. This will reject the promise and prevent an invalid param from being stored should length or checksum not match.
TESTING
packages/shared/lib/src/sdk/mod.ts
namada-interface/packages/shared/lib/src/sdk/mod.ts
Lines 21 to 40 in e160a31
yarn wasm:build
(Even though these changes are 100% TS, they are a part ofshared
lib and will not auto-reload in dev mode)yarn dev:proxy
(bypasses CORS issue with localhost)useSdk
to force a download on next refresh:namada-interface/apps/namadillo/src/hooks/useSdk.tsx
Lines 60 to 62 in e160a31
hasMaspParams()
will return false, so it will retry all of them on next refresh.yarn dev
locally (without the proxy) which will trigger theretry
behavior, it is currently configured to retry a total of 3 times with 3 second intervals in between before failingTODO
Retry
button that invokes the download again.NOTES